-
-
Notifications
You must be signed in to change notification settings - Fork 162
Improved serialization/deserialization for floating point numbers. #88
Conversation
I assume you found this with rust-fuzz/targets#99? :) |
src/ser.rs
Outdated
@@ -73,7 +73,11 @@ impl ser::Serializer for Serializer { | |||
} | |||
|
|||
fn serialize_f64(self, v: f64) -> Result<Yaml> { | |||
Ok(Yaml::Real(v.to_string())) | |||
Ok(Yaml::Real(if v.trunc() == v { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error: strict comparison of f32 or f64
--> src/ser.rs:76:26
|
76 | Ok(Yaml::Real(if v.trunc() == v {
| ^^^^^^^^^^^^^^ help: consider comparing them within some error: `(v.trunc() - v).abs() < error`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about switching to dtoa
instead? That way we avoid printing big exponents like 25e200f64
as a bunch of garbage digits.
2499999999999999754368647829910432568018032091847598320736246101108455787066727664225769304644938620168687085525975646159967957760326637205079237977314680335729196571361796923253538665328348131219210240.
@dtolnay one issue with |
From 10.2.1.4. Floating Point it looks like the correct values would be |
- deserialize/serialize inf/-inf/nan - always output decimal point for truncated floating points
c03b471
to
595c2cd
Compare
@dtolnay how's this look? |
let float: f64 = serde_yaml::from_str(&unindent(" | ||
--- | ||
.nan")).unwrap(); | ||
assert!(float.is_nan()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't compare a nan with a nan, so gotta do the is_nan
dance 💃
Looks good! I filed #89 to follow up on some odd cases. I don't think |
"50."
--(deserialize)-->yaml::Number::from(50.0)
--(serialize)-->"50"
--(deserialize)-->yaml::Number::from(50)